#526 - Provide ability to override Audio Player Controls via filter. #528
Conversation
| $markup = apply_filters( 'classifai_pre_render_post_audio_controls', false, $content, $_post, $audio_attachment_id, $audio_attachment_url ); | ||
|
|
||
| if ( false !== $markup ) { | ||
| return (string) $markup . $content; |
There was a problem hiding this comment.
Curious for your thoughts here @joshuaabenazer. The more I think about it, the more I lean towards just returning whatever custom markup someone adds using this new filter, instead of us prepending that to the post content.
For instance, if someone wanted to add a play button to the end of their content, this wouldn't allow them to do that. Since we pass the post content into the filter (as well as the full post object) thinking maybe it's up to whoever uses this filter to ensure they are appending/prepending the post content with their custom markup rather than us hardcoding that here
There was a problem hiding this comment.
@dkotter Yes, I originally did not have the content appended, but I felt like it might make sense after you posted the sample code on Slack.
It would also depend on how we word the filter too, since it is namedclassifai_pre_render_post_audio_controls, I felt it gives a sense that we are in charge of the audio only.
Now come to think of it we should just give the control to the developer as that might make it a little less confusing of what is expected. If you agree, then I can work on making that change and possibly change the filter nomenclature too(?)
I will be able to pick this up early next week though as I sign off early on Fridays.
Also originally I was just going to supply $content and $audio_attachment_url params to the filter since mostly we would need that only and the remaining can be derived if needed, but it is good to provide the other params since we have it already.
There was a problem hiding this comment.
If you agree, then I can work on making that change and possibly change the filter nomenclature too(?)
I will be able to pick this up early next week though as I sign off early on Fridays.
Yeah, this sounds good to me 👍🏻
There was a problem hiding this comment.
@joshuaabenazer Let me know if you want me to finish this off. Hoping to get a 2.2.3 release out this week and would love to include this if possible
There was a problem hiding this comment.
@dkotter got sidetracked by other priorities. I may be able to get to this on Friday or early next week. That being said I don't mind you taking over to finish this off too.
There was a problem hiding this comment.
So in looking at this again, I've changed my mind and going to merge this in as-is. Looking at the render_post_audio_controls method, it builds the audio markup and prepends that to the post content. So I think having this custom filter do the same thing (take whatever custom markup someone provides and then prepending it to the post content) is fine. If we get complaints or use cases that this doesn't support, happy to look at changing this in the future
|
Made a few PHPCS changes and some wording tweaks but tested and this works good. Left one comment to discuss but then this can get merged in |
Description of the Change
Closes #526
How to test the Change
classifai_pre_render_post_audio_controlsfilter to return audio player markup, and enqueue its related styles and scripts.Changelog Entry
Credits
Props @joshuaabenazer
Checklist: